Skip to content

Comments

fix(evm): remove system tx presence check from executor finish()#2736

Open
gakonst wants to merge 1 commit intomainfrom
klkvr/move-system-tx-validation-to-consensus
Open

fix(evm): remove system tx presence check from executor finish()#2736
gakonst wants to merge 1 commit intomainfrom
klkvr/move-system-tx-validation-to-consensus

Conversation

@gakonst
Copy link
Contributor

@gakonst gakonst commented Feb 17, 2026

Summary

Remove the BlockSection validation from TempoBlockExecutor::finish() that unconditionally required blocks to end with system transactions. This fixes eth_simulateV1 returning -32603: end-of-block system transactions not seen on every call.

Motivation

eth_simulateV1 runs user-provided transactions through TempoBlockExecutor and calls finish(), but simulated blocks never include Tempo's end-of-block system transactions (subblock signatures/metadata), so the check always failed.

System transaction presence is already validated by TempoConsensus::validate_block_pre_execution() for real blocks, making the executor check redundant.

Changes

  • Removed BlockSection::System { seen_subblocks_signatures: true } guard from TempoBlockExecutor::finish() — now delegates directly to self.inner.finish()
  • Removed test_finish_system_tx_not_seen test (no longer applicable)
  • Simplified test_finish to not require pre-setting the section

Testing

cargo test -p tempo-evm --lib block  # 23 passed
cargo clippy -p tempo-evm -p tempo-consensus -- -D warnings  # clean

Prompted by: arsenii

System transaction presence is already validated by TempoConsensus in
validate_block_pre_execution(). The redundant check in
TempoBlockExecutor::finish() broke eth_simulateV1 because simulated
blocks never include end-of-block system transactions.

Amp-Thread-ID: https://ampcode.com/threads/T-019c6b81-2dfa-729c-be92-a812fb16e22c
Co-authored-by: Amp <amp@ampcode.com>
@github-actions
Copy link

⚠️ Changelog not found.

A changelog entry is required before merging. We've generated a suggested changelog based on your changes:

Preview
---
tempo-evm: patch
---

Removed system transaction presence check from executor finish method.

Add changelog to commit this to your branch.

fn finish(
self,
) -> Result<(Self::Evm, BlockExecutionResult<Self::Receipt>), BlockExecutionError> {
if self.section
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we remove this, is this field then even useful?

this isnt documented so i dont really know how this is used

Copy link
Member

@klkvr klkvr Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this field is used in validate_tx to track section transitions:

pub(crate) fn validate_tx(
&self,
tx: &TempoTxEnvelope,
gas_used: u64,
) -> Result<BlockSection, BlockValidationError> {
// Start with processing of transaction kinds that require specific sections.
if tx.is_system_tx() {
self.validate_system_tx(tx)
} else if let Some(tx_proposer) = tx.subblock_proposer() {
match self.section {
BlockSection::GasIncentive | BlockSection::System { .. } => {
Err(BlockValidationError::msg("subblock section already passed"))
}
BlockSection::StartOfBlock | BlockSection::NonShared => {
Ok(BlockSection::SubBlock {
proposer: tx_proposer,
})
}
BlockSection::SubBlock { proposer } => {
if proposer == tx_proposer
|| !self.seen_subblocks.iter().any(|(p, _)| *p == tx_proposer)
{
Ok(BlockSection::SubBlock {
proposer: tx_proposer,
})
} else {
Err(BlockValidationError::msg(
"proposer's subblock already processed",
))
}
}
}
} else {
match self.section {
BlockSection::StartOfBlock | BlockSection::NonShared => {
if gas_used > self.non_shared_gas_left
|| (!tx.is_payment() && gas_used > self.non_payment_gas_left)
{
// Assume that this transaction wants to make use of gas incentive section
//
// This would only be possible if no non-empty subblocks were included.
Ok(BlockSection::GasIncentive)
} else {
Ok(BlockSection::NonShared)
}
}
BlockSection::SubBlock { .. } => {
// If we were just processing a subblock, assume that this transaction wants to make
// use of gas incentive section, thus concluding subblocks execution.
Ok(BlockSection::GasIncentive)
}
BlockSection::GasIncentive => Ok(BlockSection::GasIncentive),
BlockSection::System { .. } => {
trace!(target: "tempo::block", tx_hash = ?*tx.tx_hash(), "Rejecting: regular transaction after system transaction");
Err(BlockValidationError::msg(
"regular transaction can't follow system transaction",
))
}
}
}
}

so the idea for this PR is that we already check on TempoConsensus side that system tx is present:

if end_of_block_system_txs.len() != SYSTEM_TX_COUNT {
return Err(ConsensusError::Other(
"Block must contain end-of-block system txs".to_string(),
));
}

and thus executor only needs to enforce that this system tx contents are correct. given that consenus validation is skipped in simulateV1, this should unblock it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants